[various] Fixes unawaited_futures violations#4067
Merged
Merged
Conversation
Collaborator
Author
Looks like this was aspirational. Not too many failures though; I'll take a look tomorrow. |
domesticmouse
approved these changes
May 24, 2023
stuartmorgan-g
commented
May 24, 2023
|
|
||
| test('Injects script into desired target', () async { | ||
| loadWebSdk(target: target); | ||
| await loadWebSdk(target: target); |
Collaborator
Author
There was a problem hiding this comment.
This change was incorrect; the test never simulates running the callback, causing the test to hang; I'll make it unawaited with a comment.
| return (_initialization ??= _doInitialization()).catchError((dynamic _) { | ||
| // Invalidate initialization if it errors out. | ||
| _initialization = null; | ||
| }); |
Collaborator
Author
There was a problem hiding this comment.
Apparently the fact that the error wasn't being caught by the returned future was (or at least has become) intentional behavior, since there was a unit test that this change broke. I doubt very much that having _initialization not be guaranteed to be null when the future completes is intentional though, so I changed it to return the catchError wrapper, but made the wrapper rethrow after nulling _initialization.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
May 25, 2023
auto-submit Bot
pushed a commit
to flutter/flutter
that referenced
this pull request
May 25, 2023
flutter/packages@fba97fa...995bfc5 2023-05-24 engine-flutter-autoroll@skia.org Roll Flutter from f86c529 to 216605b (18 revisions) (flutter/packages#4071) 2023-05-24 stuartmorgan@google.com [various] Fixes `unawaited_futures` violations (flutter/packages#4067) 2023-05-24 48731731+phcs971@users.noreply.github.com [quick_actions] Fix iOS home screen quick actions documentation hyperlink (flutter/packages#4059) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
11 tasks
auto-submit Bot
pushed a commit
that referenced
this pull request
Jun 16, 2023
…4171) In #4067 I changed several implicitly unawaited futures to `await`, not noticing that doing so would change the timing on updating fields that were used to compute diffs for future updates. Since the update flow is unawaited at higher levels, this meant that multiple calls to update map objects in rapid succession would compute incorrect diffs due to using the wrong base objects. This fixes that by restoring the old flow using `unawaited`. Adds new tests that introduce synthetic awaits into the fake platform interface object and perform multiple updates in a row that should have different base object sets, which fail without the fix. In order to simplify adding that behavior to the fake, and to pay down technical debt in the unit tests, I replaced the old test fake that is based on method channel interception (which predates the federation of the plugin and is implicitly relying on the method channel implementation from a different package never changing) with a fake platform interface implementation (substantially simplifying the fake). Fixes flutter/flutter#128042
This was referenced Jul 7, 2023
11 tasks
11 tasks
JSUYA
added a commit
to JSUYA/plugins
that referenced
this pull request
Mar 6, 2024
Some packages require package upgrade rather than simple modification (ex: google_maps_flutter) Therefore, the rules below are temporarily ignored. Any necessary modifications can be applied in a later PR. flutter/packages#4060 - unnecessary_null_comparison flutter/packages#4067 - unawaited_futures flutter/packages#5717 - dangling_library_doc_comments - no_literal_bool_comparisons - unnecessary_library_directive - use_colored_box - use_enums - use_string_in_part_of_directives
creatorpiyush
pushed a commit
to creatorpiyush/packages
that referenced
this pull request
Jun 10, 2026
This option had been disabled to match flutter/flutter, but the reason it was disabled there was "too many false positives", mostly around animation. That doesn't apply to most packages here, and we've had a number of production bugs�especially in plugins, that use async heavily in ways that are intended to be client-awaitable�that this would have caught. This PR: - Enables the option at the repo level. - Permanently (unless the owners decide to change it) opts out `animations` and `go_router`, both of which looked like mostly or entirely false positives. - Temporarily opted out a few plugins that have a lot of violations that should be handled in their own PRs later (`camera_android_camerax`, most of `webview_flutter`). - Fixes all remaining violations. In many cases this PR is behavior-changing, replacing implicitly unawaited futures that did not seem obviously intentional with `await`ed futures, so non-test code in particular should be reviewed carefully to make sure the changes are correct. All of the changes are manual, not `fix`-generated. Part of flutter/flutter#127323
creatorpiyush
pushed a commit
to creatorpiyush/packages
that referenced
this pull request
Jun 10, 2026
…lutter#4171) In flutter#4067 I changed several implicitly unawaited futures to `await`, not noticing that doing so would change the timing on updating fields that were used to compute diffs for future updates. Since the update flow is unawaited at higher levels, this meant that multiple calls to update map objects in rapid succession would compute incorrect diffs due to using the wrong base objects. This fixes that by restoring the old flow using `unawaited`. Adds new tests that introduce synthetic awaits into the fake platform interface object and perform multiple updates in a row that should have different base object sets, which fail without the fix. In order to simplify adding that behavior to the fake, and to pay down technical debt in the unit tests, I replaced the old test fake that is based on method channel interception (which predates the federation of the plugin and is implicitly relying on the method channel implementation from a different package never changing) with a fake platform interface implementation (substantially simplifying the fake). Fixes flutter/flutter#128042
bisor0627
pushed a commit
to bisor0627/packages
that referenced
this pull request
Jun 19, 2026
This option had been disabled to match flutter/flutter, but the reason it was disabled there was "too many false positives", mostly around animation. That doesn't apply to most packages here, and we've had a number of production bugs�especially in plugins, that use async heavily in ways that are intended to be client-awaitable�that this would have caught. This PR: - Enables the option at the repo level. - Permanently (unless the owners decide to change it) opts out `animations` and `go_router`, both of which looked like mostly or entirely false positives. - Temporarily opted out a few plugins that have a lot of violations that should be handled in their own PRs later (`camera_android_camerax`, most of `webview_flutter`). - Fixes all remaining violations. In many cases this PR is behavior-changing, replacing implicitly unawaited futures that did not seem obviously intentional with `await`ed futures, so non-test code in particular should be reviewed carefully to make sure the changes are correct. All of the changes are manual, not `fix`-generated. Part of flutter/flutter#127323
bisor0627
pushed a commit
to bisor0627/packages
that referenced
this pull request
Jun 19, 2026
…lutter#4171) In flutter#4067 I changed several implicitly unawaited futures to `await`, not noticing that doing so would change the timing on updating fields that were used to compute diffs for future updates. Since the update flow is unawaited at higher levels, this meant that multiple calls to update map objects in rapid succession would compute incorrect diffs due to using the wrong base objects. This fixes that by restoring the old flow using `unawaited`. Adds new tests that introduce synthetic awaits into the fake platform interface object and perform multiple updates in a row that should have different base object sets, which fail without the fix. In order to simplify adding that behavior to the fake, and to pay down technical debt in the unit tests, I replaced the old test fake that is based on method channel interception (which predates the federation of the plugin and is implicitly relying on the method channel implementation from a different package never changing) with a fake platform interface implementation (substantially simplifying the fake). Fixes flutter/flutter#128042
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This option had been disabled to match flutter/flutter, but the reason it was disabled there was "too many false positives", mostly around animation. That doesn't apply to most packages here, and we've had a number of production bugs—especially in plugins, that use async heavily in ways that are intended to be client-awaitable—that this would have caught.
This PR:
animationsandgo_router, both of which looked like mostly or entirely false positives.camera_android_camerax, most ofwebview_flutter).In many cases this PR is behavior-changing, replacing implicitly unawaited futures that did not seem obviously intentional with
awaited futures, so non-test code in particular should be reviewed carefully to make sure the changes are correct. All of the changes are manual, notfix-generated.Part of flutter/flutter#127323
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).